Skip to content

Replace synchronized(this) with lock-free initState in LazyList#26100

Open
lrytz wants to merge 4 commits into
scala:mainfrom
lrytz:i25777
Open

Replace synchronized(this) with lock-free initState in LazyList#26100
lrytz wants to merge 4 commits into
scala:mainfrom
lrytz:i25777

Conversation

@lrytz
Copy link
Copy Markdown
Member

@lrytz lrytz commented May 19, 2026

JDK 23+ allocates ObjectMonitor instances in native memory eagerly in the single-threaded case under nested recursive synchronized blocks.

This can lead to huge memory consumption while evaluating LazyLists.

This commit replaces synchronized with a lock-free coordination.

Forward-port of scala/scala#11242.

Fixes #25777

How much have you relied on LLM-based tools in this contribution?

Moderately, for diagnosing the issue, reviewing my code, writing tests.

How was the solution tested?

  • New automated tests (including the issue's reproducer, if applicable)
  • Covered by existing tests

@lrytz lrytz requested a review from a team as a code owner May 19, 2026 20:07
@lrytz lrytz force-pushed the i25777 branch 3 times, most recently from b77e7d7 to 32723dc Compare May 20, 2026 07:52
Comment thread library-js/src/scala/collection/immutable/LazyListBase.scala Outdated
@SolalPirelli
Copy link
Copy Markdown
Contributor

Does this need to be also done for LazyListIterable, which AFAIK is a "capture checking safe" version of LazyList? (IMHO it'd be a lot more user-friendly to take whatever breaking change is necessary and not have two of these, but I guess that's been discussed before)

@lrytz
Copy link
Copy Markdown
Member Author

lrytz commented May 20, 2026

Error:  scala-library-bootstrapped: Failed binary compatibility check against org.scala-lang:scala-library:3.8.0! Found 1 potential problems (filtered 956)
Error:   * static method <clinit>()Unit in object scala.collection.immutable.LazyList does not have a correspondent in current version

Is interesting. For object definitions, the backend puts the MODULE$ initialization code into the class constructor (<clinit>) of the companion. If no such method exists, it creates one (public).

When adding a lazy val to an object, the lazyVals phase adds a @static field to the object and the moveStatic phase will create a <clinit> method, but private.

So adding a lazy val to an existing object triggers MiMa, the class initializer turns from public to private.

It's safe to ignore, the method is only invoked by the JVM and it can be private. But it seems worth aligning to avoid the MiMa flag.

@sjrd
Copy link
Copy Markdown
Member

sjrd commented May 20, 2026

IMO MiMa is just wrong here and should never report anything about <clinit> (in one direction or the other).

JDK 23+ allocates `ObjectMonitor` instances in native memory eagerly in
the single-threaded case under nested recursive synchronized blocks.

This can lead to huge memory consumption while evaluating LazyLists.

This commit replaces synchronized with a lock-free coordination.
@lrytz lrytz force-pushed the i25777 branch 2 times, most recently from b478e78 to 0135565 Compare May 20, 2026 13:13
@bishabosha
Copy link
Copy Markdown
Member

Does this need to be also done for LazyListIterable, which AFAIK is a "capture checking safe" version of LazyList? (IMHO it'd be a lot more user-friendly to take whatever breaking change is necessary and not have two of these, but I guess that's been discussed before)

its the same file copy pasted with Seq parent type removed, so yeah :)

@lrytz
Copy link
Copy Markdown
Member Author

lrytz commented May 20, 2026

This seems to pass CI now.

@natsukagami would you have time to provide the right capture annotations and unsafeAssumePure calls in LazyListIterable / LazyListIterableBase? I'm not up to speed with capture checking yet (todo!).


def InRace(t: Thread): InRace = throw new Exception("unreachable")

final class InRace private[LazyListBase] (val owner: Thread) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As suggested in the Scala 2.js version of this PR (scala-js/scala-js#5358):

Suggested change
final class InRace private[LazyListBase] (val owner: Thread) {
final class InRace private[LazyListBase] () {
throw new Exception("unreachable")
def owner: Thread = throw new Exception("unreachable")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Memory leak in LazyList + Java 25 combination

4 participants